329964 - Can't open a bookmark in the Web Panels sidebar in Places-enabled builds

Status: RESOLVED FIXED

(Firefox :: Bookmarks & History, defect, P2)

Description

[Bob Clary [:bc] (inactive)]

I have bookmarks I have configured to open in the sidebar however places appears to have killed that functionality. 

If this is intentional, please let me know so I can stop using Firefox and go back to Seamonkey.

Comment 1

[Dave Townsend [:mossop]]

*** Bug 330166 has been marked as a duplicate of this bug. ***

Comment 2

[Brett Wilson]

*** Bug 330696 has been marked as a duplicate of this bug. ***

Comment 3

[Steffen Wilberg]

This is also called Web Panels.

Also see Ben's "Web Panels in Places" thread in mozilla.dev.apps.firefox, where he asks whether we need to re-implement this in the new Places world. Please follow-up in the newsgroup.

Comment 4

[Peter Kasting]

Going to the URL on this bug and clicking one of the "Add" links results in "*** IMPLEMENT ME".

Comment 5

[(not reading, please use [e-mail] instead)]

see also http://lxr.mozilla.org/seamonkey/source/browser/base/content/browser.js#5443

Comment 6

[Asaf Romano (gone)]

Attachment: patch

No Import support yet.

Comment 7

[Asaf Romano (gone)]

Attachment: import & export

Comment 8

[Dietrich Ayala (:dietrich)]

Comment on attachment 257460 [details] [diff] [review]
patch

>Index: browser/components/places/content/bookmarkProperties.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/content/bookmarkProperties.js,v
>retrieving revision 1.32
>diff -u -p -8 -r1.32 bookmarkProperties.js
>--- browser/components/places/content/bookmarkProperties.js	5 Mar 2007 22:39:20 -0000	1.32
>+++ browser/components/places/content/bookmarkProperties.js	6 Mar 2007 02:05:01 -0000
>@@ -90,18 +92,18 @@ const LIVEMARK_CONTAINER = 2;
> 
> const ACTION_EDIT = 0;
> const ACTION_ADD = 1;
> const ACTION_ADD_WITH_ITEMS = 2;
> 
> /**
>  * Supported options:
>  * BOOKMARK_ITEM : ACTION_EDIT, ACTION_ADD
>- * BOOKMARK_FOLDER : ACTION_EDIT, ADD_WITH_ITEMS
>- * LIVEMARK_CONTAINER : ACTION_EDIT
>+ * BOOKMARK_FOLDER : ACTION_ADD, ACTION_EDIT, ADD_WITH_ITEMS
>+ * LIVEMARK_CONTAINER : ACTION_ADD, ACTION_EDIT
>  */
> 
> var BookmarkPropertiesPanel = {

is this change related to this bug?

>Index: browser/components/places/content/controller.js
>===================================================================
>RCS file: /cvsroot/mozilla/browser/components/places/content/controller.js,v
>retrieving revision 1.130
>diff -u -p -8 -r1.130 controller.js
>--- browser/components/places/content/controller.js	5 Mar 2007 22:39:21 -0000	1.130
>+++ browser/components/places/content/controller.js	6 Mar 2007 02:05:09 -0000
>   openSelectedNodeIn: function PC_openSelectedNodeIn(aWhere) {
>     var node = this._view.selectedURINode;
>-    if (node && PlacesUtils.checkURLSecurity(node))
>+    if (node && PlacesUtils.checkURLSecurity(node)) {
>+      if (aWhere == "current") {
>+        // Check whether the node is a bookmark which should be opened as a
>+        // web panel
>+        if (PlacesUtils.nodeIsBookmark(node)) {
>+          var placeURI = PlacesUtils.bookmarks.getItemURI(node.bookmarkId);
>+          if (PlacesUtils.annotations
>+                         .hasAnnotation(placeURI, LOAD_IN_SIDEBAR_ANNO)) {
>+            var w = getTopWin();
>+            if (w) {
>+              w.openWebPanel(title, node.uri);
>+              return;
>+            }
>+          }
>+        }
>+      }
>       openUILinkIn(node.uri, aWhere);
>+    }
>   },

is it necessary to have that many nested |if| statements?


>@@ -2032,16 +2048,58 @@ PlacesEditBookmarkURITransaction.prototy
>   },
> 
>   undoTransaction: function PEBUT_undoTransaction() {
>     this.bookmarks.changeBookmarkURI(this.id, this._oldURI);
>   }
> };
> 
> /**
>+ * Set/Unset Load-in-sidabr annotation
>+ */

nit: s/sidabr/sidebar/

r=me

Comment 9

[Asaf Romano (gone)]

(In reply to comment #8)
> >Index: browser/components/places/content/bookmarkProperties.js
> >===================================================================

> > 
> > const ACTION_EDIT = 0;
> > const ACTION_ADD = 1;
> > const ACTION_ADD_WITH_ITEMS = 2;
> > 
> > /**
> >  * Supported options:
> >  * BOOKMARK_ITEM : ACTION_EDIT, ACTION_ADD
> >- * BOOKMARK_FOLDER : ACTION_EDIT, ADD_WITH_ITEMS
> >- * LIVEMARK_CONTAINER : ACTION_EDIT
> >+ * BOOKMARK_FOLDER : ACTION_ADD, ACTION_EDIT, ADD_WITH_ITEMS
> >+ * LIVEMARK_CONTAINER : ACTION_ADD, ACTION_EDIT
> >  */
> > 
> > var BookmarkPropertiesPanel = {
> 
> is this change related to this bug?

No, it's a change i forgot to do in bug 357316.

Comment 10

[Asaf Romano (gone)]

Attachment: as checked in

mozilla/browser/components/places/content/bookmarkProperties.xul 1.18
mozilla/browser/components/places/content/controller.js  1.134
mozilla/browser/components/places/content/utils.js 1.20
mozilla/browser/locales/en-US/chrome/browser/places/bookmarkProperties.dtd 1.9
mozilla/toolkit/components/places/src/nsBookmarksHTML.cpp 1.32
mozilla/toolkit/components/places/src/nsNavBookmarks.h 1.32

Comment 11

[Robert Sayre]

we shouldn't be adding 50+ lines to toolkit history code without tests. do we have coverage for these changes?

Comment 12

[Asaf Romano (gone)]

do we have coverage for places import/export functionality at all?

Comment 13

[Robert Sayre]

(In reply to comment #12)
> do we have coverage for places import/export functionality at all?
> 

yikes, I don't know, but we definitely should have test coverage whenever we touch user data.

Comment 14

[Benjamin Smedberg]

Mano, are you going to be able to do some tests for this? Late breakage of import code is especially likely to go unnoticed because most of our regular testers will have been imported much earlier.

Comment 15

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to comment #13)
> (In reply to comment #12)
> > do we have coverage for places import/export functionality at all?
> > 
> 
> yikes, I don't know, but we definitely should have test coverage whenever we
> touch user data.
> 

I just tried the latest build from here:
https://bugzilla.mozilla.org/show_bug.cgi?id=329964
and Import does not work, and there are no bookmarks at all, not even the 'default' ones.  Also, NO bookmarks can be created either thru drag&drop or using Ctrl+D, a blank bookmark window shows up is the only action that appears when trying to bookmark a page. 

Comment 16

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to comment #13)
> (In reply to comment #12)
> > do we have coverage for places import/export functionality at all?
> > 
> 
> yikes, I don't know, but we definitely should have test coverage whenever we
> touch user data.
> 

I just tried the latest build from here:
http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/fxexp-win32-tbox-trunk/
and Import does not work, and there are no bookmarks at all, not even the 'default' ones.  Also, NO bookmarks can be created either thru drag&drop or using Ctrl+D, a blank bookmark window shows up is the only action that appears when trying to bookmark a page. 

Comment 17

[Gervase Markham [:gerv]]

Bug 451915 - move Firefox/Places bugs to Firefox/Bookmarks and History. Remove all bugspam from this move by filtering for the string "places-to-b-and-h".

In Thunderbird 3.0b, you do that as follows:
Tools | Message Filters
Make sure the correct account is selected. Click "New"
Conditions: Body   contains   places-to-b-and-h
Change the action to "Delete Message".
Select "Manually Run" from the dropdown at the top.
Click OK.

Select the filter in the list, make sure "Inbox" is selected at the bottom, and click "Run Now". This should delete all the bugspam. You can then delete the filter.

Gerv